Skip to content

chore: merge 6.x into main + worker-orchestrator support for services - #554

Merged
sebastiancorrea81 merged 96 commits into
mainfrom
chore/merge-6x-into-main
Sep 3, 2026
Merged

chore: merge 6.x into main + worker-orchestrator support for services#554
sebastiancorrea81 merged 96 commits into
mainfrom
chore/merge-6x-into-main

Conversation

@sebastiancorrea81

Copy link
Copy Markdown
Collaborator

Summary

Recreated PR (the original #548 was closed intentionally). Same branch, now with a full day of additional work on top.

  • Merges 6.x into main, unifying the two release lines — every 6.x release (6.11.3 through 6.23.1) and its underlying feature/fix commits are now in this branch's history.
  • nullplatform/agent worker-orchestrator (package-exec) support, generalized beyond the original single "containers" package:
    • worker_orchestrated_packages (list, default ["containers"]) — any package listed gets the agent's own ServiceAccount (IRSA) and worker_memory_limit (default 2Gi) via a per-package patch, instead of both being hardcoded to "containers" only. Fixes two real bugs found validating the aws-s3-bucket service end-to-end: the worker couldn't sts:AssumeRole (no ServiceAccount) and then OOM-adjacent-hung on tofu apply (256Mi default).
    • agent_repos_scope/agent_repos_scope_tag/agent_repos_extra replaced with a single agent_repo (list of repo#ref strings) for the legacy git-clone exec flow.
    • Deploy/DNS env vars (DNS_TYPE, DOMAIN, SERVICE_TEMPLATE, etc.) moved from the agent pod's own env to the worker's env only (breaking change — see commit 85ec91cb) since they're consumed by worker-orchestrator deploys, not the agent control loop.
    • Ported the safe, non-conflicting parts of 6.x's fix(nullplatform/agent): restore the inputs and Helm flags dropped in v6.14.0 #519: create_namespace/atomic/cleanup_on_fail on the Helm release (a failed upgrade now rolls back instead of sticking in failed with orphaned resources — hit this exact issue live today), and null-filtering in the values map before templatefile().
  • nullplatform/scope_definition and service_definition: package.artifacts[] oci_image meta defaults (registry/repository) are now overridable per module instantiation via package_oci_default_registry/package_oci_default_repository, instead of hardcoded. service_definition gained the same oci_image meta-default merge scope_definition already had.
  • nullplatform/service_definition_agent_association: added worker_orchestrator/package_slug support (mirrors scope_definition_agent_association, which already had it) — required to route a service's notification channel through package-exec instead of the legacy git-clone exec flow. Also dropped the two variables that were required even when unused in that mode.
  • nullplatform/api_key: ported 6.x's internal option (feat(api_key): add the internal option #547) — marks plumbing keys (agent, notification channels) out of the API key listing.
  • infrastructure/gcp/backend: self-provisions a log bucket (with versioning) when log_bucket is unset.
  • nullplatform/asset/{ecr,s3,docker_server}: removed an unused dimensions variable.

Test plan

  • tofu test green across every touched module (nullplatform/agent 24 tests, nullplatform/api_key 14, infrastructure/gcp/backend 26)
  • Validated end-to-end against a live EKS test cluster (logs-controller-cluster, providers-test account) for two worker-orchestrated packages:
    • containers scope: applied with worker-orchestrator enabled, published a real package revision, confirmed a worker pod picks up deploy-template paths, DNS config, and traffic-manager image.
    • aws-s3-bucket service (new): registered as a service_definition + package (oci_image artifact), wired its notification channel to package-exec, and confirmed the full create → apply → delete lifecycle against real AWS resources (S3 bucket with encryption/versioning/public-access-block, real IAM assume-role, real Terraform state in S3).

🤖 Generated with Claude Code

gdrojas and others added 30 commits August 11, 2026 15:07
…) (#493)

`terraform_data.trigger` used `triggers_replace = timestamp()`, so it replaced
on every plan and dragged `azapi_update_resource.aks_subnet_route_table` with it
through `replace_triggered_by`. Combined with the vnet AVM subnet proposing
`routeTable -> null` every plan, the azure stack never reached `No changes`
(#474) and every apply detached/re-attached the route table on a live kubenet
cluster.

Key the trigger on the values that actually matter -- the node subnet id and the
discovered route table id -- so it re-attaches only when the attachment really
changes. The other half of #474 (the vnet detaching the route table) is already
addressed by the `route_table` passthrough on `subnets_definition` (#475);
document it here as the preferred, converging approach.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…table (#494)

An AKS internal gateway sat at PROGRAMMED=False for days on a real install,
with the service-controller retrying 582 times. Two module defects, and the
first one disguises itself as the second.

`internal_azure_load_balancer_subnet` defaulted to "load_balancer". That is the
key a subnet typically has in a `subnets_definition` map, not its resource name,
so the internal gateway got annotated with a subnet that does not exist. Azure
answers a missing scope with

    403 AuthorizationFailed ... over scope '.../subnets/load_balancer'
    or the scope is invalid

which reads like missing RBAC and is not. Default to "" instead, matching the
sibling `gateway_public_azure_load_balancer_subnet` ("empty by default, in which
case Azure picks the subnet automatically"). The two variables disagreed.

Also quote the value in the values template, like the public one already is, so
an empty subnet renders `azure_load_balancer_subnet: ""` rather than a YAML null.

And fixing the name alone still 403s, for real: the aks module hardcoded
`network_contributor_role_assigned_subnet_ids` to the node subnet, so any other
subnet the cloud-provider must write into -- the one an internal LB is pinned to
-- had no permissions. Callers can now pass extra subnet IDs; the node subnet is
still granted automatically and the default is empty, so nothing changes for
existing callers.

Tests: two runs mirroring the public-subnet ones, asserting the empty default
and that a set value reaches the rendered internal block. 20 passed, 0 failed.

Needs nullplatform/helm-charts#172 to ship first: the chart emits the subnet
annotation unconditionally, so an empty value would render a null annotation
until that guard lands.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Sebastian Correa <correa.sebasti@gmail.com>
* ci: allow package/* branches and enable checks on 6.x line

* ci: enforce correct base branch per branch type (package vs 6.x)

* ci: temporarily block breaking-change commits on all branches

* chore: allow package/* branches and exempt 6.x in local pre-commit hook

* ci: run release-please on both main and 6.x lines

* ci: auto-merge release PRs from either the main or 6.x release line

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(ci): prevent script injection in auto-merge-release workflow

Move github.event.workflow_run.head_branch into env block to prevent
direct interpolation into bash script. Aligns with patterns used in
commitlint.yml and no-breaking-changes.yml.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* ci: run tofu lint on PRs targeting 6.x too

* ci: run tofu tests on PRs targeting 6.x too

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* ci: run trivy scan on the 6.x line too

* ci: run unused-declaration check on PRs targeting 6.x too

* docs: add executable rollback runbook for the temporary dual release line

---------

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… release line scheme (#504)

* fix(ci): correct release-please target-branch resolution for the dual release line scheme

release-please-action's target-branch input defaults to the repository's
default branch, not github.ref_name, so pushes to 6.x were silently
computing versions against main's history instead of 6.x's. Also hardens
the breaking-change scanner (paginate past 30 commits, also check the PR
title since this repo squash-merges using it) and hoists github.ref_name
out of two run: blocks into env vars.

* fix(docs): scope rollback runbook's commit search to main/6.x only

Paso 1's git log --all --grep picked up a false positive from a stale
pre-squash feature branch, with no way for an unsupervised agent to tell
it apart from the real main/6.x commits. Scope the search per-branch
instead, and clarify Paso 2's "repetir por cada SHA" flow accordingly.

---------

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
🤖 I have created a release *beep* *boop*
---


##
[6.11.3](v6.11.2...v6.11.3)
(2026-08-11)


### Bug Fixes

* **azure/aks_route_table:** stable trigger instead of timestamp()
([#474](#474))
([#493](#493))
([8d07ed2])
* **azure:** make the internal gateway LB subnet configurable and
grantable
([#494](#494))
([36f4540])
* **ci:** correct release-please target-branch resolution for the dual
release line scheme
([#504](#504))
([895385e])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
* ci: verify a branch actually forked from its declared base line

base-branch-check only validates that a branch's NAME is paired with the
right base — a feat/* branch created from main (instead of 6.x) still
passes that check, but merging it would smuggle every main-only change
(including the breaking change 6.x is meant to be free of) into 6.x.
This adds a job that checks the real git ancestry via merge-base.

* fix(ci): use the branch's real fork point in the rebase hint, not the historical fork point

Using $(git merge-base origin/main origin/6.x) in the remediation message
replayed every main-only commit onto 6.x -- exactly what the check exists
to prevent -- and made the check pass afterward. Use the already-computed
$MB_OTHER instead. Also derives OTHER_BRANCH from BASE_BRANCH rather than
from HEAD_BRANCH's name (avoids a vacuous pass if this ever runs on a
branch type not covered by the naming rule), drops the now-unnecessary
ci/* exemption, adds least-privilege permissions, and rewords the error
to describe the observation rather than assert a cause.

* docs(ci): clarify fork-point-check's ci/* comment

---------

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
* chore(6.x): release 6.12.0

* docs: regenerate READMEs for changed modules and update versions

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…straint (#490)

scope_definition and scope_definition_agent_association require nullplatform
>= 0.0.99, but their committed .terraform.lock.hcl still pinned 0.0.95. The
tofu-validate pre-commit hook runs `tofu init` with -lockfile=readonly, so it
could not reconcile the two and failed for anyone committing a change in
either module:

    Could not resolve provider nullplatform/nullplatform: locked provider
    registry.opentofu.org/nullplatform/nullplatform 0.0.95 does not match
    configured version constraint >= 0.0.99

CI was unaffected, so this only showed up locally.

Re-locked with `tofu providers lock` scoped to that provider, keeping the three
platforms the files already covered (linux_amd64, darwin_amd64, darwin_arm64).
Plain `tofu init -upgrade` would also have bumped http and external, which is
unrelated here, and would have narrowed the hashes to the local platform.
…ation path (#461)

The module hardcoded rbac_aad_azure_rbac_enabled = false and exposed neither
local_account_disabled nor the admin group ids, so a cluster whose local
accounts were disabled to meet a security baseline could not be expressed in
configuration. Consumers hit two problems: a plan reverts the hardening back to
the provider default, and the admin_* outputs go empty, which surfaces as
"x509: apiserver certificate is not trusted" rather than as a missing credential.

Adds local_account_disabled, azure_rbac_enabled and admin_group_object_ids, all
passed through to the upstream module. Defaults preserve today's behaviour:
local_account_disabled is null and azure_rbac_enabled keeps the previous
hardcoded false.

A precondition rejects local_account_disabled = true unless Azure RBAC or an
admin group is configured. Without one of those, no identity is authorized
against the API server, and the cluster is reachable only through an admin
kubeconfig issued beforehand — unrecoverable from configuration once that
credential stops working.

The README documents the hardened setup, including the kubelogin exec block
consumers need once the admin_* outputs are empty.
* chore(6.x): release 6.13.0

* docs: regenerate READMEs for changed modules and update versions

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…okup (#512)

data.google_container_cluster.this[0].subnetwork echoes back whatever
format the cluster was created with. When the cluster comes from
terraform-google-modules/kubernetes-engine (as in infrastructure/gcp/gke),
that's a full "projects/.../regions/.../subnetworks/NAME" path, not a
bare name — but data.google_compute_subnetwork.this only accepts a bare
name in its `name` argument, so every apply combining infrastructure/gcp/gke
with infrastructure/gcp/security failed with a 400 "Invalid value for
field 'subnetwork'" error. This is 100% reproducible, not a race
condition, and none of the module's existing override variables
(gcp_network_name, network_cidr) avoid it, since the subnetwork data
source's count doesn't depend on them.

Take the last "/"-separated segment of the cluster's subnetwork
attribute before using it, which is correct whether the value is
already a bare name or a full path.

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* chore(6.x): release 6.13.1

* docs: regenerate READMEs for changed modules and update versions

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
#515)

* feat(nullplatform/agent): require ingress templates for non-aws clouds

The k8s scope's default ingress templates are AWS-specific (ALB Ingress
annotations); on any other cloud they silently produce no working route.
Add preconditions requiring service_template, initial_ingress_path, and
blue_green_ingress_path whenever cloud_provider is not "aws", matching the
existing cross_variable_validation pattern for aws/azure requirements.

* fix(nullplatform/agent): key ingress-template requirement off INGRESS_TYPE, not cloud_provider

cloud_provider was the wrong axis — a cluster can use Istio (or not) on any
cloud, and cloud_provider != "aws" doesn't actually imply Istio routing.
Require service_template, initial_ingress_path, and blue_green_ingress_path
only when extra_envs.INGRESS_TYPE == "istio", matching the real signal that
determines which ingress mechanism the k8s scope needs to target.

* fix(nullplatform/agent): drop dead PRIVATE_DOMAIN, validate private_gateway_name for gcp/oci

PRIVATE_DOMAIN was never read anywhere in the nullplatform/scopes k8s
scope's scripts (only documented, never consumed) — removed from
cloud_config.gcp and cloud_config.oci, and dropped the now-unused
private_domain variable entirely.

private_gateway_name, by contrast, is actively read by the k8s scope's
DNS/gateway routing scripts for any private-visibility deployment — widen
its existing precondition (previously azure-only) to also require it for
gcp and oci.

* feat(nullplatform/agent): default private/public gateway names, dedupe cloud_config

private_gateway_name and public_gateway_name were duplicated identically
across azure/gcp/oci despite not being cloud-specific behavior — moved to
default_config (applies to every cloud provider) and given real defaults
("gateway-private"/"gateway-public") instead of null, matching the naming
convention already used consistently across real deployments. Dropped the
now-redundant azure/gcp/oci-scoped precondition requiring them, and cleaned
up the resulting dead cloud_config entries (empty gcp block, duplicate oci
PRIVATE_GATEWAY_NAME).

* docs(nullplatform/agent): fix stale Azure-only comments on gateway name variables

* refactor(nullplatform/agent): drop unused nrn variable

nrn was kept only for "interface parity" with other nullplatform modules
via a tflint-ignore, but the agent resolves its own scope from the API key
and never actually read it. Removed outright instead of suppressing the
lint warning — no caller-side behavior depends on it.

* fix(nullplatform/agent): remove redundant public_gateway_name precondition

public_gateway_name now defaults to "gateway-public" (same change already
applied to private_gateway_name) — the azure-only precondition requiring
it non-null is unreachable in practice, same reasoning as the
private_gateway_name precondition removed earlier in this branch.

* refactor(nullplatform/agent): reorganize variables.tf into coherent sections

Group variables by what actually gates them, and fix descriptions that had
drifted from reality:
- image_tag and cloud_provider moved into Required Variables (no default,
  same as api_key/cluster_name/tags_selectors — they were previously buried
  mid-file among optional ones).
- cluster_name's description said "EKS cluster" (AWS-only language) even
  though this module supports gcp/azure/oci too — generalized to
  "Kubernetes cluster".
- private_gateway_name/public_gateway_name moved out of "Azure
  Configuration" into their own "Gateway Configuration" section — they're
  universal (in default_config), not azure-specific.
- domain and use_account_slug's "(required when cloud_provider is 'azure')"
  comments were false — no precondition ever enforced that. Dropped the
  claim; use_account_slug moved to Agent configuration (naming behavior,
  not DNS).
- service_template/initial_ingress_path/blue_green_ingress_path/extra_envs
  moved into a new "Ingress / Networking Configuration" section instead of
  trailing after "Image Configuration", and given the same one-line leading
  comment style as every other variable in the file.

* fix(nullplatform/scope_definition_agent_association): default description non-empty

Give the notification channel a sensible default description instead of
an empty string.

---------

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
🤖 I have created a release *beep* *boop*
---


##
[6.14.0](v6.13.1...v6.14.0)
(2026-08-14)


### Features

* **nullplatform/agent:** require ingress templates for non-aws clouds
([#515](#515))
([fbb4198])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
* feat(gcp/artifact-registry): optional static service account key

Add generate_key to create a google_service_account_key and expose it via
service_account_key_base64, for Docker clients outside the cluster (e.g. a
nullplatform docker-server provider config) that can't use Workload Identity.

* fix(gcp/artifact-registry): pin the key output, add a rotation lever, document the exposure

Review follow-ups on the static service account key. No change to the default
behavior: generate_key still defaults to false and creates nothing.

- The test suite did not pin which key attribute the output exposes. Swapping
  `private_key` for `public_key` in outputs.tf — a real, also-base64, also-computed
  attribute on the same resource — kept all 7 runs green while shipping a public
  key as the Docker password, which only fails at runtime. `key_created_when_requested`
  now compares the output against `private_key` directly and runs as `apply`, since
  both sides are computed.
- `nullable = false` on `generate_key`. It was unset, so a consumer threading an
  optional root variable (`generate_key = var.maybe_key`) hit `Error: Null condition`
  at plan instead of the default.
- Added `key_rotation_token`, wired to the resource's `keepers`. GCP user-managed
  keys never expire and there was no supported way to rotate — a consumer had to
  know to run `tofu apply -replace`. Left unset by default so the key stays
  deterministic; three tests cover unset, empty and provided.
- Documented what the key costs before you enable it: the private key is stored in
  plaintext in state (`sensitive = true` redacts display, not state); the credential
  is project-scoped, so a leak can overwrite tags in every Artifact Registry
  repository in the project, not just this one; rotation is manual; and the key is
  unrecoverable after state loss because the provider only populates `private_key`
  on create, so the output silently becomes empty rather than erroring.
- Noted that `_json_key` (as opposed to `_json_key_base64`) needs `base64decode()`.
  The existing GCP stack uses that form, so this was a real footgun.
- Aligned the new usage example's `?ref=` with the rest of the README (v6.14.0);
  the 6.x merge had updated the other block and left this one behind.

Mutation-tested: the `public_key` swap, inverting the `count` guard, making
`keepers` unconditional, and dropping `sensitive` from the output each fail the
suite now. 7 runs to 10.

---------

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
Co-authored-by: Gonzalo Rojas <gonzalo.rojas@nullplatform.io>
)

* feat(gcp/gke): support Autopilot mode and flexible/spot node pools

Add autopilot_enabled to switch between a standard cluster with manually
managed node pools and a GKE Autopilot cluster. Widen node_pools to allow
either an autoscaling min/max range or a fixed node_count, and spot or
preemptible VMs per pool, with a validation guarding against setting both.
Also wire the previously-unused tags variable into cluster_resource_labels
on both submodules.

* fix(gcp/gke): add moved block, declare google-beta, cover both modes with tests

Review follow-ups. The first item is a data-loss bug on a default-config upgrade.

- Adding `count` to the pre-existing `module "gke"` moves its state address from
  `module.gke` to `module.gke[0]`, and nothing migrated it. A consumer who bumped
  only the module ref, leaving `autopilot_enabled` at its default false, would get
  a plan that DESTROYS the live cluster, every node pool and the service account,
  because the old address reads as "not in configuration" — and
  `deletion_protection_enabled` defaults to false, so nothing blocks it. Added a
  `moved` block. Verified with a state-migration harness on OpenTofu 1.10.7: the
  old shape applied, then the new shape planned `1 to add, 1 to destroy` without
  the block and `0 to add, 0 to change, 0 to destroy` with it.
- Declared `google-beta` in `providers.tf`. The Autopilot submodule creates its
  cluster with `provider = google-beta`, and provider requirements are static —
  `count = 0` does not suppress them — so it was being resolved unpinned and with
  an empty default configuration, meaning the root's credentials or
  impersonation never reached it. It was also resolving to a different major than
  `google` (6.50.0 alongside 5.45.2). Constrained to `~> 5.0` and regenerated the
  lock; both now resolve to 5.45.2 and a clean `init` succeeds, which it did not
  before this commit once the constraint was added.
- `total_min_count`/`total_max_count` are now accepted per pool, because
  `min_count`/`max_count`/`node_count` are PER ZONE and this module always creates
  regional clusters — a pool asking for `node_count = 1` in a three-zone region
  gets three nodes, so the README's own examples understated capacity and cost by
  3x. They must be set together, which is validated.
  Note the subtlety this required: a declared `optional(number)` with no default is
  present as a key holding null, and the wrapped module decides with
  `contains(keys(autoscaling.value), "total_min_count")`, not a null check
  (private-cluster/cluster.tf:558). Passing the nulls through would read as "set"
  for every pool and null out both the per-zone and total counts, leaving
  autoscaling unbounded. `local.node_pools` strips null-valued keys; a test guards it.
- Exposed `node_pools_taints`. GKE adds only labels to Spot nodes in standard
  clusters — the `cloud.google.com/gke-spot` NoSchedule taint comes solely from node
  auto-provisioning, which is not this path — so the README's spot example let any
  pod without a nodeSelector, including nullplatform system workloads, be scheduled
  onto capacity that is reclaimed on 15 seconds' notice. The standard "critical
  workloads on on-demand only" pattern was inexpressible through this module.
- README: the Architecture and Features sections claimed the module "sets up logging
  and monitoring", qualified as "(standard mode)". Both were backwards. Standard mode
  sets `logging_service = "none"` (logging DISABLED); the Autopilot submodule has no
  `logging_service` input at all and Autopilot cannot disable logging, so flipping
  the flag adds ingestion cost. Also documented that switching modes destroys the
  cluster, added `authorized_ip_ranges` to the Autopilot example (without it the
  public control-plane endpoint accepts 0.0.0.0/0), and aligned the stale `?ref=`.

Tests: the module had none and was in no `tofu-test.yml` allowlist, so the green
"All module tests passed" check was vacuous for this path. Added
`infrastructure/gcp/gke` to `test-gcp-modules` and a 13-run suite covering both
modes, the two validations, the null-stripping, and taint passthrough. The PR body
said plan-level tests were impractical because the wrapped module's internal
`google_compute_zones`/`google_container_engine_versions` need API-shaped responses
— they are reachable with `mock_data`, plus a `mock_resource` default for
`google_service_account.member`, which the provider validates.

---------

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
Co-authored-by: Gonzalo Rojas <gonzalo.rojas@nullplatform.io>
🤖 I have created a release *beep* *boop*
---


##
[6.15.0](v6.14.0...v6.15.0)
(2026-08-14)


### Features

* **gcp/artifact-registry:** optional static service account key
([#514](#514))
([aaa9674])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
* feat(gcp/backend): add module for GCS terraform state bucket

Fills a gap in the GCP module set: infrastructure/aws/backend and
infrastructure/oci/backend both have a state-bucket module, but there
was no GCP equivalent. Creates a google_storage_bucket with a random
suffix (bucket names must be globally unique across GCP), versioning
and uniform bucket-level access enabled by default, force_destroy
defaulted to false to protect state, optional customer-managed
encryption via an existing KMS key, and optional IAM member
restrictions via roles/storage.objectAdmin bindings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(gcp/backend): fix usage example version ref to match the 6.x line

Was v7.0.0 (from when this branch was based on main); this branch is
now based on 6.x, whose current release is v6.11.3.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(gcp/backend): validate inputs, lowercase the prefix, cover the security defaults

Review follow-ups on the new module. All plan-time correctness, no change to the
resource surface except two optional additions.

- `lower()` wrapped `random_id.bucket_suffix.hex`, which is already lowercase by
  construction, instead of `bucket_prefix`. An uppercase prefix planned clean and
  failed at apply with an invalid GCS bucket name.
- `kms_key_name = ""` still emitted an `encryption` block with an empty key
  instead of falling back to Google-managed encryption as documented. An empty
  string reaches the module whenever the value is wired from another module's
  output or a TF_VAR.
- No validation on `bucket_prefix` (a prefix over 46 chars pushed the name past
  the GCS 63-char limit; illegal characters and the reserved `goog`/`google`
  names passed), `storage_class`, or `public_access_prevention` (`"enforce"`
  planned fine and failed at apply, leaving the operator believing PAP was set).
- `allowed_members` was documented as restricting bucket access.
  `google_storage_bucket_iam_member` is additive: every project-level
  `roles/editor` or `roles/storage.admin` holder keeps full read on state. The
  resource choice is correct — `_iam_binding` would be authoritative and wipe
  unmanaged bindings — so the docs were the defect.
- Added optional `log_bucket` for access logging, so reads of state objects leave
  an audit trail. Closes the Trivy GCP-0077 finding on this module.
- `nullable = false` on every input with a non-null default, so an explicit
  `null` cannot bypass the default.
- Renamed `labels` to `tags`, matching gke, artifact-registry and cloud-dns.
  Free to do while the module is unreleased.

Tests: 10 runs to 26, and the suite now has teeth. The security defaults were
untested — deleting `uniform_bucket_level_access` from main.tf kept all 10 runs
green, and the provider default is false. Every new assertion was mutation-tested:
reverting each fix above, dropping either validation, and flipping the
`public_access_prevention` and `versioning_enabled` defaults each fail the suite.

Docs: added a bootstrap section (the name embeds a random suffix, so it cannot
feed a `backend "gcs"` block directly), the CMEK service-agent grant prerequisite
this module does not create, the versioning/secret-retention caveat, and the
fact that changing `bucket_prefix` replaces the bucket. Regenerated the
terraform-docs block, which also picks up the pinned provider versions the
committed lock file implies — matching artifact-registry and cloud-dns.

* docs(gcp/backend): explain why Trivy still flags logging and CMEK

Corrects the record: the previous commit claimed adding the optional `log_bucket`
closes the Trivy GCP-0077 finding. It does not. Trivy evaluates the static
configuration with default variable values, and both `log_bucket` and
`kms_key_name` default to null, so the `dynamic` blocks produce nothing and the
scanner correctly sees a bucket with neither logging nor CMEK. GCP-0077 and
GCP-0066 are still reported on this module.

Documented rather than suppressed. The repo's .trivyignore is a flat ID list with
no path scoping, so adding GCP-0077 would silence bucket-logging findings for
every module including infrastructure/aws/backend. Making either control
mandatory is not an option either: each needs a resource this module does not
create (an existing log bucket, an existing KMS key).

---------

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Gonzalo Rojas <gonzalo.rojas@nullplatform.io>
🤖 I have created a release *beep* *boop*
---


##
[6.16.0](v6.15.0...v6.16.0)
(2026-08-14)


### Features

* **gcp/backend:** add GCS terraform state bucket module
([#511](#511))
([9355c6e])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
…on (#520)

Follow-up to #512, which fixed the shape of the subnetwork reference but kept
resolving it against the configured project and region.

The parse reduced `projects/P/regions/R/subnetworks/NAME` to `NAME` and then handed
it to `google_compute_subnetwork` with `project = var.gcp_project_id` and
`region = var.gcp_region`, discarding the authoritative pair the string carried. Two
cases still 404 after #512:

- A zonal cluster. `gcp_region` must hold the ZONE, because the same variable is the
  cluster data source's `location`. The path says `regions/us-central1`; the lookup
  asks for `us-central1-a`.
- A Shared VPC subnet. The path names the host project; the lookup asks the cluster
  project.

The comment's justification for discarding them was also wrong on both counts. It
said the attribute "echoes back whatever format the cluster was created with", but
the provider normalizes any input through `RelativeLink()` before the call and reads
it back from `cluster.NetworkConfig.Subnetwork` — the field even carries
`DiffSuppressFunc: CompareSelfLinkOrResourceName` because config and read shapes
differ. So it is always the path form, which means #512 fixed a total outage for
every consumer with `cluster_name` set, not just clusters created by
terraform-google-modules. It also said `google_compute_subnetwork` "only accepts a
bare name"; the data source has an optional `self_link`, and its read path derives
project, region and name from the link.

Now one regex captures all three segments (and tolerates a self_link, since the
leading group absorbs the API prefix), with the configured project and region as the
fallback for a bare name. Replaces the double `split()` whose two lines had to keep
their index arithmetic in sync, and whose null input failed inside `locals` with
`argument must not be null`.

Also gates the data sources on whether their results are actually needed. Both were
gated only on `cluster_name != ""`, so the documented `gcp_network_name` and
`network_cidr` overrides did not avoid the reads — every consumer had to hold
container.clusters.get and compute.subnetworks.get even when supplying all derived
values, and a consumer hitting a shape the parse mishandles had no escape hatch.
`network_cidr` now skips the subnetwork read, and both overrides together skip the
cluster read as well.

Tests: 3 runs to 8. One of the three was vacuous — its two assertions were
`length(...) == 1` on values that depend only on the enable flags, so reverting the
parse left it green (verified). Nothing asserted `source_ranges` or `network`, which
are what the CIDR derivation actually produces, so a regression there would have
shipped: `source_ranges = concat([""], ...)` plans fine under mock_provider and only
fails at apply — exactly how the original bug escaped. Now asserts the derived
CIDR reaches both health-check and private-HTTPS rules, that the network reference
passes through unparsed (`google_compute_firewall.network` runs it through
`ParseGlobalFieldValue`, so a full path is valid there and must not be trimmed),
both override paths, and the self_link and bare-name shapes.

Mutation-tested: reverting to the name-only parse, ungating either data source,
inverting the network-name override precedence, and trimming the network reference
each fail the suite.

Co-authored-by: Sebastian Correa <correa.sebasti@gmail.com>
sebas_correa and others added 20 commits September 1, 2026 17:51
TRAFFIC_CONTAINER_IMAGE's assembly was duplicated verbatim in
default_config and worker_default_config. Extracted to
local.traffic_container_image, referenced by both.
Fixes a regression from the previous commit: worker_env had been
rebuilt as merge(default_config, cloud_config, extra_envs), but
default_config never carried DNS_TYPE/DOMAIN/USE_ACCOUNT_SLUG/
K8S_NAMESPACE/SERVICE_TEMPLATE/INITIAL_INGRESS_PATH/BLUE_GREEN_INGRESS_PATH
(they lived only in the now-removed worker_default_config) — so the
worker silently lost the very env vars the Istio template-path fix
depends on.

Resolution: fold those 7 keys into default_config so the agent and the
worker share one config map (all_config), instead of keeping two maps
in sync. worker_container_patch now reads env from local.all_config
directly; the separate worker_env/worker_default_config locals are
gone.

NAMESPACE is renamed to K8S_NAMESPACE (verified against a live worker
pod's actual env) since there's now only one map — this is what the
worker reads and nothing else in the agent's own args/config
depended on the literal key "NAMESPACE".

Updates the tests that encoded the old separation.
default_config/all_config (the agent pod's own configuration.values) now
carries only NP_API_KEY/TAGS/IMAGE_TAG plus cloud_config; everything the
worker needs to render a scope's k8s deployment or run traffic-management
actions (DNS_TYPE, DOMAIN, USE_ACCOUNT_SLUG, K8S_NAMESPACE,
SERVICE_TEMPLATE, INITIAL_INGRESS_PATH, BLUE_GREEN_INGRESS_PATH,
TRAFFIC_CONTAINER_IMAGE, IMAGE_PULL_SECRETS, PRIVATE_GATEWAY_NAME,
PUBLIC_GATEWAY_NAME) moves to its own worker_default_env/worker_all_config,
merged with a worker-specific cloud_config (currently just azure) and
var.extra_envs the same way all_config is. Keeping two maps in sync (rather
than sharing one, as this branch briefly had) is intentional: the agent and
the worker have different consumers and don't need each other's variables.

Also drops agent_repos_scope/agent_repos_scope_tag/agent_repos_extra and
the AGENT_REPOS arg they fed — no longer needed — and restores
local.worker_defaults (backend="kubernetes") as the base of worker_final,
which had been dropped while reworking this, leaving the worker block
without a backend default.

Updates tests to match: two obsolete scope-repo tests removed, three
rewritten for the new split (traffic-manager image now asserted in the
worker's env, not the agent's flat config; deploy/DNS vars asserted absent
from the agent's own config).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
agent_repos_scope/agent_repos_scope_tag/agent_repos_extra (and the
AGENT_REPOS arg) were dropped in the previous commit on the assumption
that every consumer had moved to worker_orchestrator — not verified,
and most of the known callers of scope_definition_agent_association
still use the legacy exec flow. Re-adds a single agent_repo variable
(with description/type) so the legacy
--command-executor-git-command-repos flow still has an escape hatch,
without restoring the old multi-repo/validation complexity that's no
longer needed.

Also cleans up stray double-blank-lines and misaligned spacing left
over from the previous commit's edits.
agent_repo is now list(string) instead of a single string, so the
legacy exec flow can point the agent at more than one repo (e.g. the
scopes repo plus a service-specific one) — joined with a comma and no
spaces, same format the old AGENT_REPOS used.
Resolves a Trivy finding on the PR (GCP-0077, MEDIUM): the state
bucket had no access logging by default. log_bucket was already an
opt-in variable, but requiring every caller to bring their own
centralized log bucket meant most installs shipped with logging off.

When log_bucket is left null/empty, the module now creates its own
dedicated log bucket and grants the project's GCS service agent
roles/storage.objectCreator on it (required for delivery — see
https://cloud.google.com/storage/docs/access-logs#delivery), so state
bucket access logging is on out of the box. Passing log_bucket still
works exactly as before and skips the self-provisioned bucket.

GCP-0066 (LOW, customer-managed encryption key) stays opt-in via
kms_key_name — enabling it by default would require every caller to
pre-create a KMS key and grant IAM on it, which is a heavier default
than warranted for a LOW-severity finding on a generic module.

Adds a log_bucket_name output and updates/extends the test suite for
the new default (26 tests, was 24).
Closes a Trivy finding (GCP-0078) the previous commit introduced: the
auto-created log bucket had no versioning, unlike tf_state. Reuses
var.versioning_enabled, same as the state bucket.
…git_repository

Mirrors the scope_definition package-artifact defaults. A service
package is typically one artifact pointing at the service's own
implementation repo, so name defaults to "impl" and type to
"git_repository" (was "oci_image", copied from scope_definition's
mirror). Every other field — slug, version, default, tags, visible_to,
and the artifact's own meta/lookup/resource_id/resource_revision_id —
stays exactly as caller-configurable as before; only meta.url/reference
need setting per release.

Verified the defaulting behavior in isolation (this module has no test
suite, and its provider is pinned below nullplatform_package/artifact
support, so a real `tofu validate`/`test` wasn't practical here).
…specific, not interchangeable

The package variable's description and the lookup-artifact data
source's comment implied digest and reference were two equivalent
ways to pin any artifact revision. Verified against the live API
against a real oci_image artifact: it's stricter than that —

  - digest must match "sha256:<64-hex>" and only oci_image accepts it
  - reference (e.g. a tag) is what git_repository uses instead
  - each type rejects the other type's field with a validation error
    ("meta has unexpected field(s): reference" / "meta.digest must
    match sha256:<64-hex>")

Clarifies both spots in scope_definition and service_definition
(mirrors of each other) to say which field belongs to which type.
…fault

The nullplatform-agent image clones service repos under /home/agent/.np,
not /root/.np — the previous default produced a cmdline pointing at a path
that doesn't exist in the running agent container.

BREAKING CHANGE: base_clone_path now defaults to /home/agent/.np. Callers
relying on the old /root/.np default must now pass it explicitly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ry other flag

The flag was rendered as "flag value" (space) instead of "flag=value" like
every sibling entry in default_args. Kubernetes passes each args[] entry as
a single argv token, and the agent image's entrypoint shell re-splits it on
that embedded space via unquoted expansion — the second word ($(AGENT_REPO))
then undergoes the shell's own command substitution (not k8s $(VAR)
substitution), fails silently, and vanishes, leaving the flag with no value.
Go's flag parser then aborts at startup: "flag needs an argument:
-command-executor-git-command-repos", crash-looping the agent pod.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…a defaults overridable

The registry/repository defaults for oci_image package artifacts were
hardcoded locals in scope_definition. Turn them into
var.package_oci_default_registry/var.package_oci_default_repository so an
implementing module can override them per instantiation, and add the same
merge to service_definition (which had no oci_image defaults at all).

Per-artifact meta still wins over the default when explicitly set — only
values omitted from meta fall back to the variable defaults.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…upport

Mirrors scope_definition_agent_association: when worker_orchestrator = true,
the channel routes package-exec to an agent that spawns the package's worker
image and runs its baked entrypoint (/app/packages/<package_slug>/entrypoint
by default), instead of the legacy git-clone exec flow. Required for a
service whose package registers an oci_image artifact to actually spawn an
np-worker pod.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ainers package

worker_container_patch (the only place serviceAccountName got set on a
worker pod) targeted package = "containers" unconditionally, so any other
worker-orchestrated package's pod fell back to the namespace's default
ServiceAccount — no IRSA identity, so sts:AssumeRole failed with "Unable to
locate credentials" (surfaced by the aws-s3-bucket service's worker trying
to assume its permissions role).

Add var.worker_orchestrated_packages (default ["containers"], preserving
current behavior) and emit one serviceAccountName-only patch per listed
package, separate from the containers-specific k8s-deployment env vars
patch. Callers add a package's slug to the list to give its worker the
agent's own ServiceAccount.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ired variables

repository_service_spec_repo and service_path had no default, forcing every
caller to set them even when worker_orchestrator = true — where they're
never read (the worker's baked entrypoint is used instead). Give both a
default of "" and add a precondition requiring repository_service_spec_repo
(the one whose absence would produce a broken cmdline) only when
worker_orchestrator = false, mirroring the existing package_slug-required-
when-true precondition.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…package

Same bug class as the serviceAccountName fix: the 2Gi memory limit only
applied via a patch targeting package = "containers", so any other
worker-orchestrated package's pod fell back to the chart's own thin default
(256Mi) — observed OOM-adjacent (255Mi/256Mi) on the aws-s3-bucket service's
worker mid-tofu-apply, which manifested as the agent's package-exec command
hanging forever with no output.

Fold the memory-limit patch into the same var.worker_orchestrated_packages
loop as serviceAccountName (new var.worker_memory_limit, default 2Gi
preserves current behavior for "containers"), leaving the containers-only
patch with just its k8s-deployment env vars.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…recated inputs)

Cherry-picks the isolated, non-conflicting pieces of 6.x commit 747cb46
("restore the inputs and Helm flags dropped in v6.14.0"). The rest of that
commit (agent_repos_scope/agent_repos_extra restoration, ingress_type) is
skipped — this branch already replaced that system with agent_repo and
handles service_template/ingress paths differently; porting it wholesale
would revert today's worker_orchestrated_packages/worker_memory_limit work.

- create_namespace/atomic/cleanup_on_fail on helm_release.agent: the provider
  defaults all three to false, so a fresh install can die on a missing
  namespace and a failed upgrade sticks in "failed" with orphaned resources
  instead of rolling back — the exact "Error upgrading chart: context
  deadline exceeded" stuck-release state hit earlier in this same session.
- nrn/private_domain restored as deprecated no-op inputs, so a caller on an
  older module version passing them doesn't fail at init.
- all_config drops null values before reaching templatefile(), which
  otherwise fails with an error naming no variable.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ports 6.x commit 1189c56 as-is — small, additive, opt-in (default null),
no overlap with anything touched on this branch. The keys this module
creates (agent, notification-channel) are platform plumbing and show up in
the API key listing alongside user-managed keys; var.internal keeps a key
out of that listing via the provider's internal mark (create-only, so
changing it replaces the key and rotates its secret).

Bumps the provider floor to ~> 0.0.101, which publishes the internal
attribute (already the version resolved elsewhere on this branch).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Their only purpose was 6.x backward compatibility (a caller on an older
module version passing them shouldn't fail at init) — this branch targets
main/7.x, a new major where breaking changes are allowed, and no caller
visible from this repo or its consumers passes either. Keeping an unused
compatibility shim for a constraint that doesn't apply to this line serves
no purpose.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@sebastiancorrea81
sebastiancorrea81 merged commit 34238fd into main Sep 3, 2026
51 checks passed
@sebastiancorrea81
sebastiancorrea81 deleted the chore/merge-6x-into-main branch September 3, 2026 13:50
release-application Bot added a commit that referenced this pull request Sep 3, 2026
🤖 I have created a release *beep* *boop*
---


##
[8.0.0](v7.1.0...v8.0.0)
(2026-09-03)


### ⚠ BREAKING CHANGES

* DNS_TYPE, DOMAIN, USE_ACCOUNT_SLUG, SERVICE_TEMPLATE,
INITIAL_INGRESS_PATH, and BLUE_GREEN_INGRESS_PATH are no longer set on
the agent pod's own env — only on the worker's. Callers relying on those
keys being present in the agent pod's configuration.values must move
that dependency to the worker.

### Miscellaneous Chores

* merge 6.x into main + worker-orchestrator support for services
([#554](#554))
([34238fd](34238fd))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
sebastiancorrea81 added a commit that referenced this pull request Sep 3, 2026
Several commits merged into main's history via #554 (the 6.x-into-main
merge) carried breaking-change markers (feat(agent)!: and BREAKING
CHANGE: footers) for real interface changes in nullplatform/agent and
service_definition_agent_association. That's causing release-please to
compute the next version as 8.0.0 (PR #555).

These changes are being treated as non-breaking for this repo's
consumption model: every consumer pins an exact `?ref=vX.Y.Z` per
module, so no automatic upgrade behavior depends on strict semver
correctness here. The actual interface changes remain fully documented
in each commit's body — only the version-bump signal is being
overridden.

Release-As: 7.2.0

Co-authored-by: sebas_correa <sebastian.correa@nullplatform.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants